Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: keep compatible with unexpected policy, revert PR: https://github.com/casbin/Casbin.NET/pull/337 #350

Merged
merged 3 commits into from
Apr 22, 2024

Conversation

sagilio0728
Copy link
Collaborator

fixed: #335

This change will be compatible with some unforeseen policy in which values count less or longer than model tokens.
For example, If your policy definition is:

p = sub, obj, act

When we attempt to add the following policy, the old logic will ignore this action and return false.
Furthermore, because the logic for checking unexpected policies is executed after the Adapter is called, the database will contain this anomalous data, and it will never be able to be loaded into memory.

"alice", "data1"
"alice", "data1", "write", "dump"

Starting from this change, the unexpected policies will be trimmed into the following expected format and can be smoothly added to the policy store. Of course, the return value will be true. Here is a simple test:

"alice", "data1", ""
"alice", "data1", "write", 

This is a more gentle approach. Conversely, there is another option where we throw an exception for all unexpected policies.
But I believe this could be a change with high risk to the user. If the user prefers a more stringent method, we can provide an option for them to choose at later versions.

QA:

@sagilio0728 sagilio0728 added the enhancement Enhancement the exist feature label Apr 21, 2024
@sagilio0728 sagilio0728 self-assigned this Apr 21, 2024
@casbin-bot
Copy link
Member

@sagilio please review

@hsluoyz hsluoyz changed the title feat: compatible with unexpected policy feat: keep compatible with unexpected policy, revert PR: #337 Apr 22, 2024
@hsluoyz hsluoyz changed the title feat: keep compatible with unexpected policy, revert PR: #337 feat: keep compatible with unexpected policy, revert PR: https://github.com/casbin/Casbin.NET/pull/337 Apr 22, 2024
@hsluoyz hsluoyz merged commit 442a59d into casbin:master Apr 22, 2024
8 checks passed
Copy link

🎉 This PR is included in version 2.6.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

sagilio0728 added a commit to sagilio0728/Casbin.NET that referenced this pull request Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement the exist feature released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot get Policy with null value in EF core Adapter.
4 participants